fix(es/minifier): preserve args at call sites of destructured bindings#11841
fix(es/minifier): preserve args at call sites of destructured bindings#11841jacobparis wants to merge 1 commit intoswc-project:mainfrom
Conversation
`visit_var_decl` only records `param_count` for `let x = init` style
declarators. Destructuring patterns (`let { x } = init`, `let [x] = init`,
etc.) are skipped, so `x.param_count` stays at the default `None`. A later
assignment to a known-arity arrow (e.g. `if (!x) x = () => true`) then sets
`param_count` to `Some(Known(0))` via the merge rule
`(None, Known(0)) => Some(Known(0))`, and `ignore_unused_args_of_call`
trims arguments at every call site of `x` accordingly.
Mirror what `report_assign_pat` already does for runtime destructuring
assignments: when the binding is anything other than a plain `Pat::Ident`,
mark every bound identifier as having an unknown param count.
Closes swc-project#11829.
🦋 Changeset detectedLatest commit: b375384 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
Pull request overview
Fixes an unsound arity-inference case in the minifier’s usage analyzer where destructured let/const bindings could incorrectly inherit a later “known” function arity and cause unused to trim arguments at call sites.
Changes:
- Mark all identifiers bound via destructuring variable declarators as having
param_count = Unknowninvisit_var_decl. - Add a regression fixture for issue #11829 verifying call arguments are preserved for destructured bindings under
unused+keep_fargs. - Add a changeset bumping
swc_coreandswc_ecma_minifierwith a patch release note.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
crates/swc_ecma_minifier/src/usage_analyzer/analyzer/mod.rs |
Conservatively sets param_count to Unknown for all ids produced by destructuring declarators, preventing stale known-arity merges that lead to argument dropping. |
crates/swc_ecma_minifier/tests/fixture/issues/11829/destructure-decl-stale-arity/input.js |
Adds minimal repro input using destructuring + conditional fallback assignment. |
crates/swc_ecma_minifier/tests/fixture/issues/11829/destructure-decl-stale-arity/output.js |
Asserts the output preserves match("hello", "world") (no arg trimming). |
crates/swc_ecma_minifier/tests/fixture/issues/11829/config.json |
Provides fixture config aligned with the #11645 unused-args fixtures (unused: true, keep_fargs: true). |
.changeset/swc-args-destructure-fix.md |
Documents the patch change for release notes/versioning. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Binary Sizes
Commit: 8f6147c |
Merging this PR will not alter performance
Comparing Footnotes
|
I ran into this issue using Slate, getting different results during next dev and next build/start, traced back to this issue. In Slate this surfaces by breaking
Editor.nodes(editor, { match: <fn> })between next dev/prod whenever a function predicate is passed to match.The core issue is that destructured bindings which are later assigned to a function have a param_count of 0 rather than unknown.
Description:
visit_var_declonly recordsparam_countforlet x = initstyle declarators. Destructuring patterns (let { x } = init,let [x] = init, etc.) fall outside the existingif let (Pat::Ident(var), Some(init)) = ...guard with no fallback, sox.param_countstays at the defaultNone. A later assignment to a known-arity arrow then setsparam_countvia the merge rule(None, Known(N)) => Some(Known(N)), andignore_unused_args_of_call(added in #11645) trims arguments at every call site ofxaccordingly.Minimal repro:
\"hello world\"main:\"undefined undefined\"(call site becomesmatch())Fix:
Mirror what
report_assign_patalready does for runtime destructuring assignments: when the binding is anything other than a plainPat::Ident, mark every bound identifier as having an unknown param count.Tests:
tests/fixture/issues/11829/destructure-decl-stale-arity/exercises the exact pattern above with the sameconfig.jsonused by the feat(es/minifier): Remove useless arguments for non inlined callee #11645 fixtures (unused: true,keep_fargs: true).mainwith diffmatch(\"hello\", \"world\")→match()and PASSES with this fix applied.tests/fixture/issues/11645/still pass — includinglogical-and-assign-stale-arity,child-scope-reassign-merge,for-of-rebind-arityand the eval/with-stmt safety tests.BREAKING CHANGE:
None.
Related issue (if exists):
Closes #11829.